fix: add check for keepout filter mask if it is not nullptr#5773
Conversation
mini-1235
left a comment
There was a problem hiding this comment.
Initially, I chose the logging type RCLCPP_WARN_THROTTLE identical to the check that in KeepoutFilter::process. But this type does't work correctly due to issue: ros2/rclcpp#2587.
Alternatively, we can temporarily use:
RCLCPP_WARN, but it fills the terminal too quickly.
RCLCPP_WARN_ONCE, this type also does not work correctly (it does not print once), but at least it provides information and with less frequency than the first option.
Replace RCLCPP_WARN_ONCE with RCLCPP_WARN_THROTTLE, when the above issue will be solved.
If you agree with the current implementation, I will also change the warning type in Keepout Filter::process
I will leave the final decision to @SteveMacenski 😄
|
@AJedancov rebase for me. We did some change recently based on a Rolling sync that need to be reflected here for CI |
Signed-off-by: AJedancov <andrei.jedancov@gmail.com>
Signed-off-by: AJedancov <andrei.jedancov@gmail.com>
692f273 to
7270aae
Compare
Codecov Report❌ Patch coverage is
... and 6 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
thanks much for this! |
…gation#5773) * fix: add check for filter mask if not nullptr Signed-off-by: AJedancov <andrei.jedancov@gmail.com> * fix: replace with throttle warning Signed-off-by: AJedancov <andrei.jedancov@gmail.com> --------- Signed-off-by: AJedancov <andrei.jedancov@gmail.com> Signed-off-by: Christopher Thompson <cthompson@metalsharkboats.com>
…gation#5773) * fix: add check for filter mask if not nullptr Signed-off-by: AJedancov <andrei.jedancov@gmail.com> * fix: replace with throttle warning Signed-off-by: AJedancov <andrei.jedancov@gmail.com> --------- Signed-off-by: AJedancov <andrei.jedancov@gmail.com>
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Description of how this change was tested
Update filter mask:
Future work that may be required in bullet points
Initially, I chose the logging type
RCLCPP_WARN_THROTTLEidentical to the check that inKeepoutFilter::process. But this type does't work correctly due to issue: Issue with RCLCPP THROTTLE logging in the plugin based approach #2587.Alternatively, we can temporarily use:
RCLCPP_WARN, but it fills the terminal too quickly.RCLCPP_WARN_ONCE, this type also does not work correctly (it does not print once), but at least it provides information and with less frequency than the first option.Replace
RCLCPP_WARN_ONCEwithRCLCPP_WARN_THROTTLE, when the above issue will be solved.If you agree with the current implementation, I will also change the warning type in
Keepout Filter::process.For Maintainers:
backport-*.